Summary

Addresses Jan's review comment on #24489. Targets that PR's branch, not master -- this is a review-feedback follow-up, not a new independent change.

Jan's complaint: the collection-exclusion mechanism in JsonPatchManager.apply() wired its own bespoke Jackson filter (mixin + filter provider, both rebuilt per call) rather than following any existing convention -- "way too complicated and entangled to be flexible or extendable."

This reworks the wiring to mirror the codebase's existing, established pattern for the same problem (skip a getter during Jackson serialization based on per-call criteria, without invoking it): org.hisp.dhis.fieldfiltering.FieldFilterService/FieldFilterMixin, already used for ?fields=.

• New JsonPatchFilterMixin (@JsonFilter) and JsonPatchExcludedPropertyFilter (SimpleBeanPropertyFilter), shaped like FieldFilterMixin/FieldFilterSimpleBeanPropertyFilter.
• JsonPatchManager caches one mixin-bound ObjectMapper copy per entity type (ConcurrentHashMap<Class<?>, ObjectMapper>, built lazily via computeIfAbsent) instead of rebuilding the binding on every apply() call. findExcludableNonOwnerCollections (the actual exclusion rule) is unchanged -- it was already schema-driven and generic, not UserRole-specific.
• The mixin is scoped per-realClass, not bound to Object.class globally. FieldFilterMixin's own filter is path-aware, so a global binding is safe for it; ours matches by property name only. A global binding was tried first here and found, in review, to silently strip Sharing.users/Sharing.userGroups (present on every IdentifiableObject) whenever a same-named top-level collection was excluded -- e.g. a scalar UserRole PATCH would wipe sharing.users. Caught before merge, never released. Scoping per-class makes that cross-type collision structurally impossible. Regression test: testUserRoleSharingUsersSurviveScalarPatch.
• Answers Jan's second question (should an explicit patch to an excluded property throw?): no -- findExcludableNonOwnerCollections already un-excludes a property referenced by a patch path, so it falls back to full (slower, correct) hydration rather than being silently dropped. Replaced the previous testUserRoleUsersPathPatchDoesNotThrow (asserted only assertNotNull, would pass either way) with testUserRoleUsersPathPatchFallsBackToFullHydration, which asserts Hibernate.isInitialized(...) actually flips to true, mutation-tested against the fallback logic.

No functional change to PATCH behavior beyond fixing the Object.class regression introduced and caught within this same review cycle.

Separately (not in this PR): David asked whether PUT has the same issue -- confirmed it doesn't (AbstractCrudController.putJsonObject parses the request body into a fresh transient object, never round-trips the persisted entity through Jackson). A dedicated PUT perf test to prove that under real load is tracked as a follow-up.

Test plan

• mvn spotless:apply clean
• JsonPatchExcludedPropertyFilterTest (new, unit): 2/2 passing
• JsonPatchManagerTest (dhis-test-integration): 14/14 passing (13 original + testUserRoleSharingUsersSurviveScalarPatch)
• UserControllerTest (dhis-test-web-api): 52/52 passing, unaffected
• dhis-test-performance module: mvn test-compile passes (Gatling UserRolesPerformanceTest untouched)
• Mutation-tested: the Object.class collision bug, the fallback-not-silently-dropped behavior, and the filter-omission mechanism itself were each confirmed to fail for the right reason when deliberately broken, then reverted

AI Assisted

🤖 Generated with Claude Code